Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #699 +/- ##
==========================================
- Coverage 91.83% 91.76% -0.07%
==========================================
Files 44 45 +1
Lines 6781 6887 +106
==========================================
+ Hits 6227 6320 +93
- Misses 554 567 +13
|
|
ready for a first pass of review |
giovp
changed the title
|
ready for review |
|
Thank you @giovp for the PR, I will review now.
I can't think of other places right now, so I think we are good with the current improvement.
I agree. |
|
|
||
| @nb.njit(parallel=False, nopython=True) | ||
| def _create_slices_and_translation( | ||
| min_values: nb.types.Array[nb.float64, nb.float64], |
There was a problem hiding this comment.
I think nb.types.Array[nb.float64, np.float64] may be incorrect and that the correct version is nb.types.Array(nb.float64, 2, 'C'). But I am not sure because pre-commit doesn't complain, so maybe both syntaxes are allowed.
There was a problem hiding this comment.
Mine is wrong. Are you sure about your typing? What looks strange to me is that types like nb.types.Array[nb.float64, nb.int64] would not have a meaning. I tried using just nb.types.Array and pre-commit works, maybe this is the way to go.
There was a problem hiding this comment.
I removed the dtype, I'd merge.
|
@giovp I finished reviewing; I applied some minor changes like simplified the docs and added an extra test. |
2 participants
Trying to vectorize the tiling in the dataloader PR, realized some improvements should be added separately.
With this PR, I enable the vectorization of
bounding_box_queryfor all elements.This means that it is now possible to pass an array of bounding boxes (and not just one).
TODO:
@LucaMarconato @melonora do you have any suggestion of when this could be used to replace current implementations across the projects? A clear use case (and the motivation for this contribution) is the dataloader, see #687 for the speedup, but I wonder if there are other places where this is useful.
This also is the groundwork for eventual update to the dataloader, being able to return batches of all elements.
considering that this PR is already getting too large, I would postpone the vectorization of polygon query.